feature: Add support for ShipModul Miniplex Latest Firmware #234
Conversation
ShipModul now uses C and R in their PSMDST message to represent a message being sent or received.
ShipModul now includes an R for receive data in the PSMDST sentence.
This change checks for a new field 'R' in the PSMDST message. This field has been added to the ShipModul Miniplex firmware and indicates that the messages is received from the ST network.
The newest firmware of ShipModul Miniplex adds 'R' after the PSMDST to indicate that the message has been received from the ST network.
test/seatalk.js
Outdated
| @@ -53,7 +53,7 @@ const should = chai.Should() | |||
| chai.use(require('chai-things')) | |||
|
|
|||
| describe('seatalk', () => { | |||
| ;['$PSMDST,', '$STALK,'].forEach((prefix) => { | |||
| ;['$PSMDST', '$PSMDST_R', '$STALK,'].forEach((prefix) => { | |||
There was a problem hiding this comment.
If I understood this PR correctly this test is not really relevant: it now tests the normal input format, but for PSMDST_R. Shouldn't we actually add a test where input does indeed have the extra R in the data?
Does the parser ever see the C data?
There was a problem hiding this comment.
I thought this test was testing all of the typical input formats. It previously had both $PSMDST and $STALK which are typical from various devices. The new Miniplex firmware adds the _R for all received data so this test just adds that option as well.
Also, the _C field may appear at the parser but I cannot imagine these would ever need to be parsed since they are sent messages meant to go onto the ST network.
There was a problem hiding this comment.
Several different things:
- You added support to a new data format, where the first (optional) data field has value
R. That looks like it works, but none of the tests exercise it - you are running the tests that were already in place, with exact the same data fields as previously, noRfield, just for an additional prefix - the tests do not pass for
PSMDSTprefix because you deleted the trailing comma in the prefix value (see the diff above, it is highlighted) - the tests do not pass for
PSMDST_Rprefix because there is no trailing comma in the new prefix value
There was a problem hiding this comment.
This should be corrected now.
|
Is this PR still alive? |
|
Apologies for the slow response. |
|
Still there? |
|
Yes still here. Just haven't had time to review yet. |
tkurki
changed the title
|
If you have any more details than "latest firmware", like the exact version when the R was added and links to Miniplex documentation feel free to add to the PR description or as a comment here. This PR will be linked from the release notes, so somebody might even check details here. |
The ShipModul Miniplex has added a field to the PSMDST message to indicate sending to 'C' or receiving from 'R' the SeaTalk network. This change adds support for the new field by looking for the 'R' field then parsing the data accordingly.